[HLSL Options] Remove select-validator option#7423
Conversation
tex3d
left a comment
tex3d
left a comment
There was a problem hiding this comment.
We need a way to update tests with an option to prevent downlevel validator testing. Some of these tests should be revisited more carefully for their intended purpose after adding the external validator path option and global environment variable override.
| // CHECK-NOT: error | ||
|
|
||
| // RUN: %dxc -T vs_6_8 -select-validator internal %s | FileCheck %s | ||
| // RUN: %dxc -T vs_6_8 %s | FileCheck %s |
There was a problem hiding this comment.
This now matches the first RUN line, so it seems to be pointless.
This test should probably be re-thought-through since the assumptions in the comments are now out of date.
There was a problem hiding this comment.
Understood, removed that Run Line. Cleaned up the comments too.
Fully intend to use the proposed dxil_dll_path option to augment these tests when I get around to that in a separate PR.
tex3d
left a comment
tex3d
left a comment
There was a problem hiding this comment.
The option test needs to be moved under the DXC path instead of Parser.
There was a problem hiding this comment.
I just realized that this is in the wrong location. This is a folder of clang c/cpp parser tests, not for testing the argument parsing. This whole folder is disabled for the DXC project, so this test will not run.
Check lit.local.cfg under Parser for:
config.suffixes = [] # HLSL Change - Disable lit suites.
This should be moved under the DXC folder instead.
pow2clk
left a comment
pow2clk
left a comment
There was a problem hiding this comment.
I have a suggestion, but you can take or leave it.
| // Test that the deprecated option, select-validator, doesn't work. | ||
| // RUN: not %dxc -E main -T vs_6_7 -select-validator internal %s 2>&1 | FileCheck %s | ||
|
|
||
| // CHECK: dxc failed : Unknown argument: '-select-validator' |
There was a problem hiding this comment.
I wasn't going to suggest we might include a bespoke error message since the option was hidden all along. For some reason, this test makes me think to at least mention the possibility of a warning that the flag is ignored and a mention of the replacement environment variable. I don't mind either way.
3 participants
This PR removes the select-validator option. It is being deprecated, and it wasn't ever officially documented.
Fixes #7419